Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix improper detection and handling of html_safe buffer in CacheHelper #2080

Merged
merged 3 commits into from Jul 24, 2011

Conversation

lhahne
Copy link

@lhahne lhahne commented Jul 15, 2011

This fixes the latest incarnation of #1537 where CacheHelper might try to use splice! for a html_safe buffer which converts the buffer unsafe. This change has been tested against Rails's tests and slim-lang which originally raised this issue.

@spastorino
Copy link
Contributor

We need a test case in order to merge this.

@lhahne
Copy link
Author

lhahne commented Jul 15, 2011

I think test_fragment_caching in caching_tests.rb already tests the bahavior of this function. In addition, even larger changes to this function, such as 114b5e4, have been accepted without specific tests. Anyways, I'll see if I can write a test case over the weekend.

@spastorino
Copy link
Contributor

@lhahne the commit you're pointing fixes some previously committed failing tests.

@lhahne
Copy link
Author

lhahne commented Jul 15, 2011

Ok. I know a way to break the old code so I can write some tests once I discover how to change the buffer in tests.

Lauri Hahne added 2 commits July 17, 2011 18:42
The output_buffer returned by CacheHelper should be html_safe if the original buffer is html_safe.
@lhahne
Copy link
Author

lhahne commented Jul 17, 2011

Ok. I added two tests one of which fails against the current 3-0-stable. After my fix, both tests pass. In addition, I modified CacheBuffer so that any possible new output_buffer is of the original type and made the tests reflect this.

@kommen
Copy link
Contributor

kommen commented Jul 19, 2011

This seems to be related to #2150

@spastorino
Copy link
Contributor

Closed in PR 2219

@spastorino spastorino closed this Jul 23, 2011
@spastorino spastorino reopened this Jul 23, 2011
@spastorino
Copy link
Contributor

We need to backport PR 2219 to 3-0-stable

@kommen
Copy link
Contributor

kommen commented Jul 23, 2011

@spastorino in #2219 the regression fixed there is introduced by 03d01ec, which is not in 3-0-stable. So merging this pull request will suffice (assuming we're not backporting 03d01ec to 3-0-stable as well).

Though I think we should update the cache helper test to match the test from master.

spastorino added a commit that referenced this pull request Jul 24, 2011
Fix improper detection and handling of html_safe buffer in CacheHelper
@spastorino spastorino merged commit eead13f into rails:3-0-stable Jul 24, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants